fix(native): make colorScheme.set move every reader of the scheme - #415
Open
YevheniiKotyrlo wants to merge 5 commits into
Open
fix(native): make colorScheme.set move every reader of the scheme#415YevheniiKotyrlo wants to merge 5 commits into
YevheniiKotyrlo wants to merge 5 commits into
Conversation
`colorScheme.set()` moved only this library's observable, so the class layer and React Native's own readers disagreed. `useColorScheme()` and every prop-valued colour read `Appearance`; `dark:` utilities read the observable. An app calling the documented setter moved one and not the other, and rendered a light canvas under dark chrome. Writing both in the one call is the whole fix. It does not try to make a direct `Appearance.setColorScheme()` visible to the class layer: that writer emits no event, and the class layer is push-based, so nothing short of a notification can move an already-mounted element.
There were two sources of truth for the scheme with different null semantics. `colorScheme.get()` coalesces through Appearance to a definite value; the class layer read the raw observable. The observable holds null at rest and after `set(null)`, so `prefers-color-scheme: light` and `dark` both failed while `get()` reported light — the element fell through to its unconditional rule. That is the same two-readers-disagree defect this branch is named for, one function along, and it is reachable through the setter the branch just changed. The tests are rewritten around what each one actually pins. The repaint case duplicated media-query.test.tsx byte for byte and is gone; the OS-event case stays, relabelled as the guard it is for Appearance.addChangeListener. The write-through assertion now checks the argument rather than the resulting cache, which passed under every mutation because get() falls back to Appearance. The fixture is three-way so "matched neither branch" is distinguishable from "matched light" — the failure above is invisible to a two-colour fixture.
YevheniiKotyrlo
marked this pull request as draft
August 15, 2026 14:35
The write-through moved two of the three readers. React Native's
setColorScheme assigns Appearance's cache and calls the native module;
the only eventEmitter.emit("change") in Libraries/Utilities/Appearance.js
sits inside the native `appearanceChanged` handler. So a write the
platform does not echo back moves getColorScheme() and notifies nobody —
and every documented way to track the scheme, useColorScheme included, is
useSyncExternalStore over addChangeListener.
colorScheme.set now announces the change on the same device event the
platform uses, so Appearance itself performs the cache write and the emit
exactly as it does for an OS change. DeviceEventEmitter is a public
react-native export and `appearanceChanged` is the event Appearance
subscribes to through NativeEventEmitter, which registers on that same
emitter.
Guarded on the cache having actually moved, so this reports a change and
never invents one: where there is no native Appearance module the write
is a no-op and both reads are null, and a redundant set of the scheme
already in force stays silent, matching the observable's own equality
guard.
The suite now fakes Libraries/Utilities/NativeAppearance rather than
replacing Appearance itself, so the cache, the change event, the
"unspecified" coercion and their ordering are react-native's own instead
of a transcription of them. That is what lets the OS-change test drive
the real platform path, and what makes the subscriber claim measurable
rather than argued.
Deriving the announcement from Appearance's cache made it depend on the
one expression react-native changed at 0.86. `setColorScheme` writes that
cache from the requested value on 0.86+; before it, from
`toColorScheme(NativeAppearance.getColorScheme())` — a read-back that is
stale on both platforms, because Android posts the night-mode switch to
the UI thread through `UiThreadUtil.runOnUiThread` (`postDelayed(r, 0)`)
and iOS never assigns `_currentColorScheme` in `setColorScheme:`. So the
cache-derived guard suppressed its own emit on every react-native below
0.86 and left subscribers to the platform echo, and on 0.86 it broadcast
`{colorScheme: null}` for `set(null)` — a scheme no reader can render.
The announcement now carries what the caller asked for, so it says the
same thing on every version in the declared peer range, and only a
resolved scheme is announced. Every other member of ColorSchemeName is a
hand-back rather than a scheme — null and undefined before 0.86, the
literal "unspecified" from 0.86 on — and only the OS knows what one
resolves to; its own echo delivers that.
Two suites cover the two cache-write rules, both driving the setter
against a platform that applies the write on a later turn, through an
explicit flush seam rather than a timer. The 0.81 suite runs the
installed Appearance over an asynchronous NativeAppearance; the 0.86 one
transcribes the four functions of that version's Appearance.js, which
cannot be installed beside it, and keeps react-native's own
NativeEventEmitter registration so what reaches its cache is what
reaches the real one.
YevheniiKotyrlo
marked this pull request as ready for review
August 15, 2026 19:13
`colorScheme.get()` and the `prefers-color-scheme` evaluator both resolved what
the scheme channel holds with `?? Appearance.getColorScheme() ?? "light"`. That
chain fires only on a nullish value, and `"unspecified"` — 0.86's spelling of
"follow the system", where 0.81 spells `null` — is not nullish. It passes
straight through.
A reader handed the literal matches neither `prefers-color-scheme: dark` nor
`: light`, so an app that asks to follow a dark system loses every
scheme-conditional class rather than falling back to one. On Android nothing
repairs that until the user toggles the system theme, because AppearanceModule
emits only when the resolved scheme changes.
`resolveColorScheme` accepts a resolved scheme and rejects everything else,
rather than naming the members it must reject. That is what makes it total: a
future release can add another "no scheme yet" spelling and it keeps answering
correctly, where a deny-list would silently gain a third hole. Both readers call
it, because the class layer and the prop layer disagreeing about the scheme is
the defect — the two copies it replaces had already drifted into being wrong
together.
The existing `set('unspecified')` test stepped past the window, asserting only
after the platform echo repairs it. The new test asserts inside it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
colorScheme.set()moves this library's observable and nothing else, so the two halves of an app's theming disagree.The class layer (
dark:utilities,@media (prefers-color-scheme)) reads the observable. React Native's own readers —useColorScheme()and every prop-valued colour — readAppearance. An app calling the documented setter moves the first and not the second, and renders a light canvas under dark chrome.This is a native-only change, and "every reader" is a claim about the native runtime.
src/web/api.tsxalready intends to write through toAppearance, and on web that call is a hardTypeError— a pre-existing defect this deliberately does not close, described in full in the last section.Fix
Three parts, all about the same thing — the library had two sources of truth for the scheme, and moving one of them told nobody.
The setter reaches all three readers, which are three separate channels: the class layer reads the observable,
useColorScheme()readsAppearance's cache, and every store built the documented way is subscribed throughAppearance.addChangeListener.And the class layer resolves the scheme the way
colorScheme.get()already does.get()coalesces throughAppearanceto a definite value;conditions/media-query.tsread the raw observable. That observable holdsnullat rest and afterset(null), soprefers-color-scheme: lightanddarkboth failed whileget()reportedlight, and the element fell through to its unconditional rule. Same defect as above, one function along, and reachable through the setter this PR changes:Why the announcement goes through
DeviceEventEmitterBecause that is the channel
Appearanceitself listens on, and React Native provides no other way in.Libraries/Utilities/Appearance.jshas two write paths and only one of them emits.setColorSchemeassignsstate.appearanceand calls the native module; the soleeventEmitter.emit('change', …)sits inside theappearanceChangedhandler registered ingetState(). So a write the platform does not echo back movesgetColorScheme()and notifies nobody — anduseColorSchemeisuseSyncExternalStore(addChangeListener, getColorScheme), so with no event it is never told to re-read. Neither is any other store built the same way, which is every documented way to track the scheme.That handler is registered through
new NativeEventEmitter(NativeAppearance).addListener('appearanceChanged', …), andNativeEventEmitter.addListenerregisters onRCTDeviceEventEmitter— its own comment says so ("all native events are fired via a globalRCTDeviceEventEmitter").DeviceEventEmitteris that emitter, exported from thereact-nativeroot (index.js, andtypes/index.d.ts). So emitting there hands the value toAppearance, which performs its own cache write and its ownchangeemit, exactly as it does for an OS change. The library invents no notification and reimplements none ofAppearance's behaviour.Why it carries the requested scheme rather than a read of the cache
Because
setColorSchemewrites that cache differently on either side of react-native 0.86, and the declared peer range (react-native >= 0.81) spans the change.Before 0.86 that read-back is stale on both platforms, because the platform applies the write on a later turn:
AppearanceModule.setColorSchemewraps the night-mode switch inUiThreadUtil.runOnUiThread {}, andrunOnUiThreadismainHandler.postDelayed(runnable, 0): always posted, never run inline.getColorScheme()still answers from the configuration in force.RCTAppearance.mm'sgetColorSchemereturns_currentColorScheme, which is assigned atinitand insideappearanceChanged:and never bysetColorScheme:— that method setswindow.overrideUserInterfaceStyleand nothing else.So a guard that reads the cache back concludes "nothing moved" on every react-native below 0.86 and suppresses its own emit; subscribers are left to the platform echo, which is exactly where they were before. From 0.86 the same guard fires — including on
set(null), where the cache goesnulland the announcement broadcasts{colorScheme: null}to every subscriber, tellinguseColorScheme()the app has no scheme at all.Keying on the requested scheme takes the version out of the question: the announcement says the same thing on every react-native in the peer range, because it never asks what the platform did with the write.
Only a resolved scheme is announced. Every other member of
ColorSchemeNameis a hand-back rather than a scheme, and that type moved at the same release — 0.81 declares'light' | 'dark' | null | undefined, 0.86 declares'light' | 'dark' | 'unspecified', so on the current release"unspecified"is the type-legal way to hand the scheme back andnullis not in the type at all. Only the OS knows what a hand-back resolves to. Announcing the request itself would put a value inAppearance's cache that no reader can render, and on 0.81"unspecified"tripstoColorScheme's invariant outright; the platform's own echo delivers the resolved scheme instead, exactly as it does for an OS change.previousis what keeps a set of the scheme already in force silent.The guard is well-typed on both, which is worth saying because
ColorSchemeNameinverted at the same release: comparing against the two scheme literals narrows identically under'light' | 'dark' | null | undefinedand under'light' | 'dark' | 'unspecified', so the line adds no error to atscrun on either.yarn typecheckhere runs against the 0.81 pin; the 0.86 union was checked by replaying the expression against it directly.Nothing in
src/compileris touched or reachable. The compiler emits the["=", "prefers-color-scheme", "dark"]condition tuple and never resolves it — its only import fromreact-nativeis thePlatformOSTypetype — so both halves of this change are confined to the native runtime that evaluates the tuple.One notification per
colorScheme.set, and the guard is not what bounds it. The emit drivesAppearance'schange, which drives this library's own subscription (reactivity.ts:222), which callscolorSchemeObs.set(event.colorScheme)with the value the line above already wrote — andobservable.set'sObject.isearly return (reactivity.ts:73-93) is what makes that second write notify nobody. Removing the guard entirely still terminates cleanly (column C below): it announces redundantly, it does not recurse.The real home for this is
Appearance.setColorSchemeitself, which should announce a cache it just moved. This is the library-side close; I am happy to take it upstream to react-native as well if you would rather have it there.Known limits, precisely
A platform that echoes the write back delivers two
changeevents with the same value. Measured as["dark", "dark"]for onecolorScheme.set("dark")followed by the echo, in both new suites. iOS setsoverrideUserInterfaceStyle, which fires a trait changeRCTAppearancere-emits; Android'ssetDefaultNightModereachesonConfigurationChanged. Both dedup against their own last emission, and neither can see aDeviceEventEmitter.emitmade in JS, so the echo still arrives.useSyncExternalStorebails on an identical snapshot, souseColorSchemere-renders once and any other subscriber sees an idempotent repeat. React Native makes no dedup guarantee on this event in the first place.A hand-back followed by a set of the scheme the stale cache happens to hold announces nothing. Measured against the 0.86 model:
set("dark"), thenset("unspecified"), thenset("light")before the platform echo lands, leaves subscribers on"dark"whileAppearance.getColorScheme()already reads"light".previousis read from a cache thatsetColorScheme's own read-back can move without notifying anybody, so it is not reliably what subscribers last heard. Closing it means the setter tracking what it last announced — state it does not otherwise need — so I have left it open rather than add that unasked.I have not measured this on hardware. The mechanism is read off react-native's own source (0.81.4 and 0.86.0, JS and both native modules) and measured in the suite; not on a device.
A direct
Appearance.setColorScheme()still does not move an already-mounted element. That is unchanged by this PR and is a different defect: the observable is seeded once at import and never re-read, so a fresh mount after a direct write is stale too — no notification would fix that, and no pull would either. Making it work means the class layer subscribing toAppearancerather than mirroring it, which is a change to how the observable is constructed and a separate question from this one.Reading
Appearancethrough on every observableget()is the obvious way to try, and it is a trap worth recording:get()on a function-init observable assigns the cached value without notifying, whilerun()'s equality guard compares against that same cache. A read landing between a change and its notification swallows the notification, permanently. I measured that as two elements with the same class rendering different colours. It does not apply to the observable as it stands — seeded with a value, it is static — but it is why the read-through is not the shortcut it looks like.Tests
Eighteen, across three files. Each column below reverts or alters one half of the change; every test is killed by at least one, except the last, which is the floor by design.
color-scheme-appearance.test.tsxuseColorSchemeis movesget()set(null)hands the scheme backcolor-scheme-appearance-async.test.tsxset(null)announces no scheme of its owncolor-scheme-appearance-rn-0-86.test.tsxset(null)broadcasts no null schemeset('unspecified')broadcasts no literalColumn D is a cache-derived announcement, which is what makes the read-back dependency measurable rather than argued: it passes every test in the original suite and fails four here. Column E is the same guard without the resolved-scheme test — the one mutation that reaches only the 0.86 literal.
The 0.81 suites fake
Libraries/Utilities/NativeAppearance, notAppearance. Under the jest presetTurboModuleRegistry.get("Appearance")is null, so the real module takes its absent-native branch — every readnull,setColorSchemea no-op, and noappearanceChangedlistener registered — which is why it cannot express the behaviour under test. Faking the one module it is missing leaves the realAppearance.jsrunning, so its cache, itschangeemit, itsunspecifiedcoercion and their ordering are react-native's own rather than a transcription of them.color-scheme-appearance-async.test.tsxis that same instrument over a native module that applies the write on a later turn, through an explicit flush seam rather than a timer, which is what a device does.color-scheme-appearance-rn-0-86.test.tsxis the one file that transcribes rather than runs: 0.86'sAppearance.jscannot be installed beside the 0.81 this repo pins, so its four functions are transcribed and quoted, and theappearanceChangedregistration is still react-native's ownNativeEventEmitter— what reaches that cache is what reaches the real one. Transcribing the version that cannot be installed, and only that, is the whole of the fakery.The write-through test asserts the argument passed to
setColorScheme, not the resulting cache. Asserting the cache passes under every mutation, becauseget()falls back toAppearance.getColorScheme()and either writer alone satisfies it.The subscriber tests render
useSyncExternalStore(addChangeListener, getColorScheme)— the exact shape ofuseColorScheme, which cannot itself be used becausereact-native/jest/setup.jsreplaces it withjest.fn(() => "light").The
prefers-color-schemefixture is three-way — unconditional green,lightblue,darkred — so "matched neither branch" is distinguishable from "matched light". Theset(null)defect is invisible to a two-colour fixture.Full suite on Windows, stable across four runs:
2 failed, 4 skipped, 56 passed, 58 of 62 totalsuites and3 failed, 21 skipped, 1067 passed, 1091 totaltests, against56 of 60/1057 passed, 1081 totalbefore this commit — exactly the ten new tests, no change in failures. The three that fail are thesrc/__tests__/babel/*path suites, unrelated to this change and fixed by #390.yarn typecheckandyarn lintboth exit 0.Second commit —
"unspecified"was leaking through the same resolutionThe first commit makes this package's own
setmove every reader. Reviewing it turned up a second hole in the resolution it leans on, reproducible on its own.Both readers resolved the scheme with
?? Appearance.getColorScheme() ?? "light". That chain fires only on a nullish value, and"unspecified"is not nullish — it is 0.86's spelling of "follow the system", where 0.81 spellsnull. So the literal passes straight through to a reader.A reader handed it matches neither
prefers-color-scheme: darknor: light, so an app that asks to follow a dark system loses every scheme-conditional class rather than falling back to one. On Android nothing repairs that until the user toggles the system theme, becauseAppearanceModuleemits only when the resolved scheme changes.The existing
set('unspecified')test could not see it: it asserts only after the platform echo, which repairs the value. The new test asserts inside that window, and fails on the parent commit withExpected: not "unspecified".resolveColorSchemeaccepts a resolved scheme and rejects everything else, rather than naming the members it must reject. That is what makes it total — a future release can add another "no scheme yet" spelling and it keeps answering correctly, where a deny-list would silently gain a third hole. It is also why nothing in it compares against"unspecified", which is outside theColorSchemeNamethe pinned react-native declares.Both readers call the one function, because the class layer and the prop layer disagreeing about the scheme is the defect itself — and the two copies it replaces had already drifted into being wrong together. The
media-query.tscopy even carried the comment "The same resolution the publiccolorScheme.get()uses", which is the invariant this makes structural instead of conventional.Note
This changes the observable behaviour of a public API, which
CONTRIBUTING.mdasks be discussed in an issue first. Happy to move it to one if you would rather — I opened it as a PR because the change and its reproduction are easier to read as a diff.Separately, the web half of this API is broken
src/web/api.tsx:76callsAppearance.setColorScheme(name), andreact-native-web@0.21.1does not implement it — itsAppearanceexports exactlygetColorSchemeandaddChangeListener. So that call is aTypeErrorfor the first caller.Nothing in the repo can see it:
src/web/api.tsx:8importsAppearancefrom"react-native", so TypeScript resolves RN's.d.ts, which does declaresetColorScheme, and the swap to react-native-web happens at bundler resolution. There are no runtime tests undersrc/web/**at all. It predates this change (aeb0085).I have deliberately not fixed it here, because it is not a missing line — it is a missing concept. A browser will not let JavaScript override
prefers-color-scheme; react-native-web has nosetColorSchemebecause there is nothing for it to do. So closing it means deciding whatcolorScheme.setmeans on web, and there are three different answers:@media (prefers-color-scheme), sodark:follows the library rather than the browser. That is a feature, not a fix.That is your call rather than mine, and it changes what a public API promises on a platform this PR does not otherwise touch — so it wants its own PR and probably its own issue. Happy to send whichever of the three you want.